Implement auto-recover functionality [B: 1403] #483

droberts-ctrlo · 2024-11-24T10:37:03Z

No description provided.

This may change

Update for bug in `setDate` (in `set-field-values`)

abeverley

Great work Dave, this is looking really good. I've made a few minor comments so far. I'd also like to test it in practice, but I'm getting Uncaught (in promise) TypeError: crypto.subtle is undefined - any ideas?

src/frontend/components/form-group/autosave/lib/autosaveBase.ts

lib/GADS.pm

src/frontend/components/modal/modals/curval/index.js

src/frontend/components/modal/modals/curval/lib/component.js

Also moved ID for current record out of Curval condition

- Added checks to ensure encryption is available for Autorecover - Improved encrypted storage testing - Added NullStorage to disable storage without having to search through all code - Added tests for NullStorage, AppStorage, and GADSStorage classes - Updated test definitions to allow for crypto testing As noted above the modal and field components will not have recovery enabled should encryption be unavailable within storage; this is in order to meet standards for ISO:27001 (DARE is mandatory) - the `setValue` function in the `curval` component should never be called when this is the case, but in case it is, the Null Receiver pattern is used (hence the `NullStorage`, which is a full no-op storage solution for our use case).

droberts-ctrlo · 2025-01-03T14:34:14Z

.github/workflows/tests.yml

@@ -178,7 +178,7 @@ jobs:

    strategy:
      matrix:
-        node-version: [18.x, 20.x]
+        node-version: [18.x, 20.x, 22.x]


Update to use all currently supported node versions

droberts-ctrlo · 2025-01-03T14:34:40Z

.github/workflows/tests.yml

@@ -187,6 +187,7 @@ jobs:
        uses: actions/setup-node@v4
        with:
          node-version: ${{ matrix.node-version }}
-          cache: 'yarn'
+          cache: 'npm'
+      - run: npx update-browserslist-db@latest


Cache and browsers list for 18.x was "complaining"

droberts-ctrlo · 2025-01-03T14:35:02Z

jest.config.js

@@ -94,6 +94,9 @@ const config = {
    "^validation$": "<rootDir>/src/frontend/js/lib/validation",
    "^logging$": "<rootDir>/src/frontend/js/lib/logging",
    "^util/(.*)$": "<rootDir>/src/frontend/js/lib/util/$1",
+    "^components/(.*)$": "<rootDir>/src/frontend/components/$1",


Mapping was inconsistent between this file and tsconfig.json

droberts-ctrlo · 2025-01-03T14:35:27Z

lib/GADS/API.pm

@@ -1448,6 +1448,17 @@ any ['get', 'post'] => '/api/users' => require_any_role [qw/useradmin superadmin
    return encode_json $return;
 };

+get '/api/get_key' => require_login sub {


Moved to API route for consistency

droberts-ctrlo · 2025-01-03T14:37:12Z

src/frontend/components/button/lib/cancel-button.ts

+    $el.on('click', async () => {
+        const href = $el.data('href');
+        await clearSavedFormValues($el.closest('form'));        
+        if (href)


This should only work for buttons, and should have the HREF value set (to make the button act like a link where used) - if it isn't, just go back in the history.

droberts-ctrlo · 2025-01-03T14:43:16Z

src/frontend/components/form-group/input/lib/documentComponent.ts

@@ -83,7 +84,7 @@ class DocumentComponent {

    async handleAjaxUpload(uri: string, csrf_token: string, file: File) {
        try {
-            if (!file) throw this.showException(new Error('No file provided'));
+            if (!file) this.showException(new Error('No file provided'));


Incorrect statement usage

droberts-ctrlo · 2025-01-03T14:44:00Z

src/frontend/components/form-group/input/lib/documentComponent.ts

@@ -181,5 +184,7 @@ class DocumentComponent {
 * @param {JQuery<HTMLElement> | HTMLElement} el The element to attach the document component to
 */
 export default function documentComponent(el: JQuery<HTMLElement> | HTMLElement) {
-    Promise.all([(new DocumentComponent(el)).init()]);


We actually need a component to be returned here now

droberts-ctrlo · 2025-01-03T14:47:05Z

src/frontend/js/lib/util/filedrag/index.ts

All that's changed here is the change to check for jQuery being defined so all the tests pass as expected

droberts-ctrlo · 2025-01-03T14:47:44Z

src/frontend/js/lib/util/gadsStorage/lib/AppStorage.ts

Abstract factory to create the relevant storage provider

droberts-ctrlo · 2025-01-03T14:48:27Z

src/frontend/js/lib/util/gadsStorage/lib/GadsStorage.test.ts

These tests are skipped on Node 18

Need to rethink error handling (i.e. `crypto` unavailable etc)

droberts-ctrlo · 2025-01-08T11:29:40Z

src/frontend/components/form-group/autosave/index.js

Not entirely sure what the difference here is, but it works like this (as opposed to the previous version) so I'm not worried.

droberts-ctrlo · 2025-01-08T11:38:44Z

src/frontend/js/lib/util/common.test.ts

These tests are related to the common utility library and had to be changed due to the changes to said library.

See the comment on the library for reasoning for this change being included here

droberts-ctrlo · 2025-01-08T11:41:19Z

src/frontend/js/lib/util/common.ts

This involved importing the globals.definitions into the packaged code—this is bad practice and meant that some functions were being replaced by stubs and/or mocks.

Whilst this has not had any effect at this point, it will become a source of bugs and/or errors going forward, and required removal.

Said behaviour was only placed in for previous requirement that has since been removed

droberts-ctrlo · 2025-01-09T10:49:06Z

src/frontend/components/form-group/autosave/lib/component.js

+
+    // For each field, when it changes save the value to the local storage
+    $field.on('change', async function () {
+      const recovering = await self.storage.getItem('recovering');


Easiest way to ensure we're not writing data when we don't need to during a recovery

droberts-ctrlo · 2025-01-09T10:51:07Z

src/frontend/js/lib/util/gadsStorage/lib/GadsStorage.ts

+    }
+
+    async setItem(key: string, value: string) {
+        if(await this.getItem('recovering')) return;


We turn off writing if we're performing a recovery to prevent extra write operations—this is more to prevent the odd curval error with dropdowns.

It's felt it's more sensible to do this here, rather than search through all the code and try to work out where to put the check (and repeat it ad infinitum)

It was found that due to the curval recovery working on events, it was sometimes triggering a write when one isn't required.

Test condition was returning true in unit tests meaning tests failed.

droberts-ctrlo added 13 commits November 24, 2024 10:36

Reset and rerun of commit

ec64c0b

Unit test failure fix

39498e3

JSTree and Test fixes

de0bcfd

Fix for textarea not setting values correctly

ef3db97

Fix for set-field-values not working with different date formats

f31e3f6

Updated storage to ignore empty values

d0f0de6

Merge branch 'dev' into autosave-reset

12c606a

Styling and error handling

d852b03

Set max height of modal

fba4f8a

This may change

Added overflow to modal

e7ab764

Update setdate

a227963

Update for bug in `setDate` (in `set-field-values`)

Updated to *remove* clone and allow empty values to be saved

68fc58b

Updated for better erroring of curvals

25dea35

droberts-ctrlo changed the title ~~Reset and rerun of commit~~ Reset and rerun of commit [B: 1403] Dec 2, 2024

droberts-ctrlo marked this pull request as draft December 2, 2024 14:53

droberts-ctrlo added 3 commits December 3, 2024 09:25

Update to erroring

70a447d

Fix for error not displaying correctly in curvals

041ba36

Clearer feedback

73fd8a5

droberts-ctrlo marked this pull request as ready for review December 16, 2024 10:36

abeverley changed the title ~~Reset and rerun of commit [B: 1403]~~ Implement auto-recover functionality [B: 1403] Dec 19, 2024

abeverley reviewed Dec 22, 2024

View reviewed changes

droberts-ctrlo added 7 commits December 27, 2024 15:34

Moved get_key into API route

a13063b

Updated code to call correct endpoint for key

10f3ea0

Also moved ID for current record out of Curval condition

Updated for clearer feedback on curvals

d649a50

Forgot feedback for successful validation

48b7d9b

Updated node tests

ac6e033

Updated GADSStorage tests as they are incompatible with Node 16

b5579e4

droberts-ctrlo commented Jan 3, 2025

View reviewed changes

Added basic JSDoc to GadsStorage

2b85d32

droberts-ctrlo and others added 3 commits January 8, 2025 10:11

Fixed autorecover (#27)

d21d155

Removed error in restore

73c1c88

Need to rethink error handling (i.e. `crypto` unavailable etc)

Re-added error handling using different method

c9f845b

droberts-ctrlo commented Jan 8, 2025

View reviewed changes

Removed import for testing definitions

4f101b5

droberts-ctrlo commented Jan 8, 2025

View reviewed changes

droberts-ctrlo added 2 commits January 9, 2025 10:43

Disable writing to restore values during recovery

9cd46bb

Removed erroneous clone behaviour

7ac8772

Said behaviour was only placed in for previous requirement that has since been removed

droberts-ctrlo commented Jan 9, 2025

View reviewed changes

droberts-ctrlo added 4 commits January 9, 2025 12:55

Fix to recovery overwrites

06baf02

It was found that due to the curval recovery working on events, it was sometimes triggering a write when one isn't required.

Removed test condition in GadsStorage

1f261a5

Test condition was returning true in unit tests meaning tests failed.

Fixed wording

33e3896

Fixed wording

9ffb2ee

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement auto-recover functionality [B: 1403] #483

Implement auto-recover functionality [B: 1403] #483

droberts-ctrlo commented Nov 24, 2024

abeverley left a comment

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 3, 2025

droberts-ctrlo Jan 8, 2025

droberts-ctrlo Jan 8, 2025

droberts-ctrlo Jan 8, 2025

droberts-ctrlo Jan 9, 2025

droberts-ctrlo Jan 9, 2025

Implement auto-recover functionality [B: 1403] #483

Are you sure you want to change the base?

Implement auto-recover functionality [B: 1403] #483

Conversation

droberts-ctrlo commented Nov 24, 2024

abeverley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment